Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Subject prefix #3422

Merged
merged 4 commits into from
Feb 5, 2024
Merged

Subject prefix #3422

merged 4 commits into from
Feb 5, 2024
+14 −1

Conversation

LaVibeX
Copy link
Contributor

@LaVibeX LaVibeX commented Jan 31, 2024 •
edited
Loading

Description

  • Email Subject Prefix is Hardcoded rightnow .subject("[Dependency-Track] " + notification.getTitle()),
  • This is a solution to allow the User to modify the Subject Prefix to its will .subject(smtpPrefix + " " + notification.getTitle() )

This modification would be available in Administration>Configuration>Email
and "[Dependency-Track]" would be the first default value.

If the User change the Subject Prefix, it will need to press Update button in order to save the changes.
Subject Prefix is not a required field so leaving the input box in blank is also an option.

This new feature helps to distinguish between several instances of Dependency-Track, e.g. prod and qual systems.

Default value: [Dependency-Track]

image

Modified Value: [Dependency-Track TEST ®]

image

Modified Value: " "

image

This PR depends on its corresponding PR in the frontend repository
DependencyTrack/frontend#720

Addressed Issue

This fixes #1823

Additional Details

Check if smtpPrefix is null in order to clean the value to " "
smtpPrefix = smtpPrefix == null ? " " : smtpPrefix;

This PR is supervised by @lukas-braune.

Checklist

  • I have read and understand the contributing guidelines
  • This PR fixes a defect, and I have provided tests to verify that the fix is effective
  • This PR implements an enhancement, and I have provided tests to verify that it works as intended
  • This PR introduces changes to the database model, and I have added corresponding update logic
  • This PR introduces new or alters existing behavior, and I have updated the documentation accordingly

Signed-off-by: Andres Tito [email protected]

@LaVibeX LaVibeX mentioned this pull request Feb 2, 2024
Merged
5 tasks
@LaVibeX
EMAIL SUBJECT PREFIX NOT HARDCODED ANYMORE
7ffb4ce 
Add Email Subject Prefix dynamic option to email sender,
it will take value from Subject prefix input box added
in Frontend.

Follows the same variable convention.
defaultPropertyValue will be [Dependency-Track]

Signed-off-by: Andres Tito <[email protected]>
@LaVibeX
CLEAN NULL SUBJECT PREFIX
af3542a 
Add condition to clean value if User choose not to have a Subject Prefix

Signed-off-by: Andres Tito <[email protected]>
@LaVibeX
Change variable smtpPrefix to emailSubjectPrefix
172a374 
The prefix is not really tied to the SMTP protocol.
If other email delivery methods are added in the future,
the prefix might still be valid to use, but should not be called
smtpPrefix.

Signed-off-by: Andres Tito <[email protected]>
@nscuro nscuro added the enhancement New feature or request label Feb 4, 2024
@nscuro nscuro added this to the 4.11 milestone Feb 4, 2024
nscuro
nscuro approved these changes Feb 4, 2024
View reviewed changes
Copy link
Member

@nscuro nscuro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @LaVibeX!

@nscuro
Copy link
Member

nscuro commented Feb 5, 2024

Looks like this broke SendMailPublisherTest. Do you mind having a look @LaVibeX?

@LaVibeX
Add Config Properties to SendMailPublischerTest and Fix blank space
19b7e5b 
Remove space on SendMailPublisher.java and
add EMAIL_PREFIX Properties to SendMailPublisherTest.java

Signed-off-by: Andres Tito <[email protected]>
@LaVibeX
Copy link
Contributor Author

LaVibeX commented Feb 5, 2024 •
edited
Loading

Locally, I ran the tests after my commit and it finish without any errors.
Here in the CI, I notice that the error is related to ProjectResourceTest due to a Condition TimeOut.

Could it be possible to run the test again @nscuro ?
image

Looks like this broke SendMailPublisherTest. Do you mind having a look @LaVibeX?

@nscuro
Copy link
Member

nscuro commented Feb 5, 2024

Triggered another test run.

@nscuro nscuro merged commit 11eb35f into DependencyTrack:master Feb 5, 2024
9 checks passed
@girtsn
Copy link

girtsn commented Feb 6, 2024

it says that it implements #1823
from the ticket "add the notification subject configuration to the notification template and use same params as there exists for content"
is it possible to use variables now as in body of the notification (e.g. severity / others)? the description of this ticket seems to be about adding a static prefix only?

@nscuro
Copy link
Member

nscuro commented Feb 7, 2024 •
edited
Loading

You are right @girtsn, I reopened the issue. Thanks for pointing that out, much appreciated!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@valentijnscholten valentijnscholten valentijnscholten requested changes

@nscuro nscuro nscuro approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
4.11
Development

Successfully merging this pull request may close these issues.

Notifications subjects - add additional information (e.g. Project/Component/Severity) into Subject string
4 participants
@LaVibeX @nscuro @girtsn @valentijnscholten